gh-152063: Cache the offset of the inline values 'valid' byte in LOAD_ATTR by Aniketsy · Pull Request #153122 · python/cpython

Aniketsy · 2026-07-05T12:32:55Z

Fixes #152063

Issue: The LOAD_ATTR_INSTANCE_VALUE instruction can be sped up by storing the offset of the valid field in the cache. #152063

markshannon

The changes to _LOAD_ATTR_INSTANCE_VALUE and the specializer look good.
There should be no reason to increase the cache size for LOAD_ATTR, there is already spare space in LOAD_ATTR_INSTANCE_VALUE

markshannon · 2026-07-06T09:50:57Z

 typedef struct {
    _Py_BackoffCounter counter;
    uint16_t type_version[2];
+    uint16_t value_offset;


This shouldn't need changing

markshannon · 2026-07-06T09:55:05Z

        macro(LOAD_ATTR) =
            _SPECIALIZE_LOAD_ATTR +
-            unused/8 +
+            unused/9 +


The cache is already large enough.
If you reduce the size of the unused cache in LOAD_ATTR_INSTANCE_VALUE by one, the other specializations don't need to change.

markshannon · 2026-07-06T10:01:19Z

-    cache->index = (uint16_t)offset;
-    write_u32(cache->version, tp_version);
+    if (base_op == LOAD_ATTR) {
+        Py_ssize_t valid_offset = (char *)&values->valid - (char *)owner;


Suggested change

Py_ssize_t valid_offset = (char *)&values->valid - (char *)owner;

Py_ssize_t validity_offset = (char *)&values->valid - (char *)owner;

valid_offset sounds like a bool for whether the offset is valid.

bedevere-app · 2026-07-06T10:05:32Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Aniketsy · 2026-07-07T07:06:50Z

@markshannon thanks for the review, could you please help me with this patch, after applying these changes

aniket@DESKTOP-074O80J:/mnt/d/cpython/cpython$ git diff Include/internal/pycore_code.h
diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h
index b4170340b2..fd8ca313e3 100644
--- a/Include/internal/pycore_code.h
+++ b/Include/internal/pycore_code.h
@@ -108,7 +108,6 @@ typedef struct {
 typedef struct {
     _Py_BackoffCounter counter;
     uint16_t type_version[2];
-    uint16_t value_offset;
     union {
         uint16_t keys_version[2];
         uint16_t dict_offset;
@@ -125,7 +124,7 @@ typedef struct {
 typedef struct {
     _Py_BackoffCounter counter;
     uint16_t version[2];
-    uint16_t value_offset;
+    uint16_t validity_offset;
     uint16_t index;
 } _PyLoadAttrInstanceValueCache;

aniket@DESKTOP-074O80J:/mnt/d/cpython/cpython$ git diff Python/bytecodes.c
diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index e9d3d5f161..fe86c92d51 100644
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -2866,7 +2866,7 @@ dummy_func(

         macro(LOAD_ATTR) =
             _SPECIALIZE_LOAD_ATTR +
-            unused/9 +
+            unused/8 +
             _LOAD_ATTR;

         op(_GUARD_TYPE_VERSION, (type_version/2, owner -- owner)) {
@@ -2890,12 +2890,13 @@ dummy_func(
             EXIT_IF(tp != (PyTypeObject *)type);
         }

-        op(_CHECK_MANAGED_OBJECT_HAS_VALUES, (value_offset/1, owner -- owner)) {
+        op(_CHECK_MANAGED_OBJECT_HAS_VALUES, (validity_offset/1, owner -- owner)) {
             PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner);
             assert(Py_TYPE(owner_o)->tp_dictoffset < 0);
             assert(Py_TYPE(owner_o)->tp_flags & Py_TPFLAGS_INLINE_VALUES);
-            uint8_t *valid_ptr = (uint8_t *)owner_o + value_offset;
-            EXIT_IF(!FT_ATOMIC_LOAD_UINT8(*valid_ptr));
+            assert((char *)&_PyObject_InlineValues(owner_o)->valid ==
+                   (char *)owner_o + validity_offset);
+            EXIT_IF(!FT_ATOMIC_LOAD_UINT8(*((uint8_t *)owner_o + validity_offset)));
         }

         op(_LOAD_ATTR_INSTANCE_VALUE, (offset/1, owner -- attr, o)) {
@@ -2955,7 +2956,7 @@ dummy_func(
             unused/1 +
             _LOAD_ATTR_MODULE +
             POP_TOP +
-            unused/6 +
+            unused/5 +
             _PUSH_NULL_CONDITIONAL;

         op(_LOAD_ATTR_WITH_HINT, (hint/1, owner -- attr, o)) {
@@ -3004,7 +3005,7 @@ dummy_func(
             _GUARD_TYPE_VERSION +
             _LOAD_ATTR_WITH_HINT +
             POP_TOP +
-            unused/6 +
+            unused/5 +
             _PUSH_NULL_CONDITIONAL;

         op(_LOAD_ATTR_SLOT, (index/1, owner -- attr, o)) {
@@ -3030,7 +3031,7 @@ dummy_func(
             _GUARD_TYPE_VERSION +
             _LOAD_ATTR_SLOT +  // NOTE: This action may also deopt
             POP_TOP +
-            unused/6 +
+            unused/5 +
             _PUSH_NULL_CONDITIONAL;

         op(_CHECK_ATTR_CLASS, (type_version/2, owner -- owner)) {
@@ -3052,7 +3053,7 @@ dummy_func(
             unused/1 +
             _RECORD_TOS +
             _CHECK_ATTR_CLASS +
-            unused/3 +
+            unused/2 +
             _LOAD_ATTR_CLASS +
             _PUSH_NULL_CONDITIONAL;

@@ -3060,7 +3061,6 @@ dummy_func(
             unused/1 +
             _RECORD_TOS +
             _GUARD_TYPE_VERSION +
-            unused/1 +
             _CHECK_ATTR_CLASS +
             _LOAD_ATTR_CLASS +
             _PUSH_NULL_CONDITIONAL;
@@ -3084,7 +3084,6 @@ dummy_func(
             _RECORD_TOS_TYPE +
             _GUARD_TYPE_VERSION +
             _CHECK_PEP_523 +
-            unused/1 +
             _LOAD_ATTR_PROPERTY_FRAME +
             _SAVE_RETURN_OFFSET +
             _PUSH_FRAME;
@@ -3113,7 +3112,6 @@ dummy_func(
             _RECORD_TOS_TYPE +
             _GUARD_TYPE_VERSION +
             _CHECK_PEP_523 +
-            unused/1 +
             _LOAD_ATTR_GETATTRIBUTE_OVERRIDDEN_FRAME +
             _SAVE_RETURN_OFFSET +
             _PUSH_FRAME;
@@ -4280,7 +4278,7 @@ dummy_func(
             unused/1 +
             _RECORD_TOS_TYPE +
             _GUARD_TYPE_VERSION +
-            unused/3 +
+            unused/2 +
             _LOAD_ATTR_METHOD_NO_DICT;

         op(_LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES, (descr/4, owner -- attr)) {
@@ -4312,7 +4310,7 @@ dummy_func(
             unused/1 +
             _RECORD_TOS_TYPE +
             _GUARD_TYPE_VERSION +
-            unused/3 +
+            unused/2 +
             _LOAD_ATTR_NONDESCRIPTOR_NO_DICT;

         op(_CHECK_ATTR_METHOD_LAZY_DICT, (dictoffset/1, owner -- owner)) {
@@ -4336,7 +4334,6 @@ dummy_func(
             unused/1 +
             _RECORD_TOS_TYPE +
             _GUARD_TYPE_VERSION +
-            unused/1 +
             _CHECK_ATTR_METHOD_LAZY_DICT +
             unused/1 +
             _LOAD_ATTR_METHOD_LAZY_DICT;
aniket@DESKTOP-074O80J:/mnt/d/cpython/cpython$ git diff Python/specialize.c
diff --git a/Python/specialize.c b/Python/specialize.c
index 7593de78af..9873d032a6 100644
--- a/Python/specialize.c
+++ b/Python/specialize.c
@@ -655,14 +655,14 @@ specialize_dict_access_inline(
         return 0;
     }
     if (base_op == LOAD_ATTR) {
-        Py_ssize_t valid_offset = (char *)&values->valid - (char *)owner;
-        if (valid_offset != (uint16_t)valid_offset) {
+        Py_ssize_t validity_offset = (char *)&values->valid - (char *)owner;
+        if (validity_offset != (uint16_t)validity_offset) {
             SPECIALIZATION_FAIL(base_op, SPEC_FAIL_OUT_OF_RANGE);
             return 0;
         }
         _PyLoadAttrInstanceValueCache *iv_cache =
             (_PyLoadAttrInstanceValueCache *)cache;
-        iv_cache->value_offset = (uint16_t)valid_offset;
+        iv_cache->validity_offset = (uint16_t)validity_offset;
         iv_cache->index = (uint16_t)offset;
         write_u32(iv_cache->version, tp_version);
     }
aniket@DESKTOP-074O80J:/mnt/d/cpython/cpython$

getting this error while building..

ation -fvisibility=hidden  -I./Include/internal -I./Include/internal/mimalloc  -I. -I./Include    -DPy_BUILD_CORE -o Python/context.o Python/context.c
In file included from ./Include/internal/pycore_backoff.h:12,
                 from Python/ceval.h:6,
                 from Python/ceval.c:3:
Python/generated_cases.c.h: In function ‘_PyEval_EvalFrameDefault’:
Python/generated_cases.c.h:8977:13: error: static assertion failed: "incorrect cache size"
 8977 |             static_assert(INLINE_CACHE_ENTRIES_LOAD_ATTR == 10, "incorrect cache size");
      |             ^~~~~~~~~~~~~
Python/generated_cases.c.h:9170:13: error: static assertion failed: "incorrect cache size"
 9170 |             static_assert(INLINE_CACHE_ENTRIES_LOAD_ATTR == 10, "incorrect cache size");
      |             ^~~~~~~~~~~~~
Python/generated_cases.c.h:9370:13: error: static assertion failed: "incorrect cache size"
 9370 |             static_assert(INLINE_CACHE_ENTRIES_LOAD_ATTR == 10, "incorrect cache size");
      |             ^~~~~~~~~~~~~
make: *** [Makefile:3363: Python/ceval.o] Error 1
make: *** Waiting for unfinished jobs....

i was kind of stuck from yesterday, so thought to take help from you 😊 . Thank you !

markshannon · 2026-07-07T08:26:35Z

It means that you've changed either the size of some of the LOAD_ATTR specializations or the size of the _PyLoadMethodCache struct.
Neither should change.

markshannon · 2026-07-07T08:28:27Z

If you increase the size of the _CHECK_MANAGED_OBJECT_HAS_VALUES uop, you'll need to reduce the number of unused cache entries by the same amount to keep the overall size the same

Cache the offset of the inline values 'valid' byte in LOAD_ATTR

82516ed

Aniketsy requested a review from markshannon as a code owner July 5, 2026 12:32

bedevere-app Bot mentioned this pull request Jul 5, 2026

The LOAD_ATTR_INSTANCE_VALUE instruction can be sped up by storing the offset of the valid field in the cache. #152063

Open

bedevere-app Bot added the awaiting review label Jul 5, 2026

Aniketsy marked this pull request as draft July 5, 2026 12:58

bedevere-app Bot removed the awaiting review label Jul 5, 2026

markshannon requested changes Jul 6, 2026

View reviewed changes

bedevere-app Bot added the awaiting changes label Jul 6, 2026

markshannon added the skip news label Jul 6, 2026

Aniketsy added 2 commits July 7, 2026 11:34

resolve conflicts

0badb77

Address review: keep _PyLoadMethodCache and cache sizes unchanged.

728d242

Aniketsy marked this pull request as ready for review July 7, 2026 18:42

bedevere-app Bot added awaiting review and removed awaiting changes labels Jul 7, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

gh-152063: Cache the offset of the inline values 'valid' byte in LOAD_ATTR#153122

gh-152063: Cache the offset of the inline values 'valid' byte in LOAD_ATTR#153122
Aniketsy wants to merge 3 commits into
python:mainfrom
Aniketsy:fix-152063

Aniketsy commented Jul 5, 2026 •

edited by bedevere-app Bot

Loading

Uh oh!

markshannon left a comment

Uh oh!

markshannon Jul 6, 2026

Uh oh!

markshannon Jul 6, 2026

Uh oh!

markshannon Jul 6, 2026

Uh oh!

bedevere-app Bot commented Jul 6, 2026

Uh oh!

Aniketsy commented Jul 7, 2026

Uh oh!

markshannon commented Jul 7, 2026

Uh oh!

markshannon commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	Py_ssize_t valid_offset = (char )&values->valid - (char )owner;
	Py_ssize_t validity_offset = (char )&values->valid - (char )owner;

Uh oh!

Uh oh!

Conversation

Aniketsy commented Jul 5, 2026 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markshannon left a comment

Choose a reason for hiding this comment

Uh oh!

markshannon Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

markshannon Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

markshannon Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

bedevere-app Bot commented Jul 6, 2026

Uh oh!

Aniketsy commented Jul 7, 2026

Uh oh!

markshannon commented Jul 7, 2026

Uh oh!

markshannon commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Aniketsy commented Jul 5, 2026 •

edited by bedevere-app Bot

Loading